Skip to content

Docs: Document the phase model and diagnose body side effects - #62

Merged
phranck merged 4 commits into
mainfrom
issue/58-render-phase-docs
Jul 21, 2026
Merged

Docs: Document the phase model and diagnose body side effects#62
phranck merged 4 commits into
mainfrom
issue/58-render-phase-docs

Conversation

@phranck

@phranck phranck commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Closes #58 and completes #13 (last sub-issue). Design spec: .claude/openspec/render-phase-effect-commit.md (status set to Implemented in this PR).

Summary

  • Diagnose unsupported user side effects inside body: a traversal window on AppState reports same-thread invalidations raised while the render loop traverses the tree into RuntimeDiagnostics (identity-tagged, deduplicated per frame); the invalidation is still honored, and background tasks plus committed effect actions stay silent
  • Rewrite RenderCycle.md for the phase-separated pipeline: per-pass collectors, the traversal diagnostics window, the frame commit step, corrected onAppear timing (fires at commit, never during traversal), and change-driven onPreferenceChange semantics
  • Add the conceptual "Render Phases and the Frame Commit" section: phases and their guarantees, the collector/pending-record split with the SwiftUI analogy, the one-question classification rule, and what a discarded pass guarantees
  • Mark the OpenSpec as implemented

Test plan

  • ./scripts/test-linux.sh green on macOS (native, Swift 6.0.3) and Linux (Docker) — 1315 tests, only the pre-existing [P0-09] Preserve subtree liveness across EquatableView cache hits #14 marker remains
  • New: BodySideEffectDiagnosticTests — state and status-bar mutations during traversal are diagnosed once per frame; committed effect mutations stay silent
  • DocC builds without diagnostics
  • No public API change (manifest untouched)

phranck added 4 commits July 22, 2026 00:52
- Add a traversal window to AppState: main-thread invalidations raised while the render loop traverses the view tree report through a violation handler; background tasks and post-commit effect actions stay silent
- Route violations into RuntimeDiagnostics with the mutating subtree's identity, deduplicated per frame
- Honor the invalidation itself so rendering stays consistent (diagnose, never swallow)
- Add a conceptual "Render Phases and the Frame Commit" section: phases, the collector/pending-record split with the SwiftUI analogy, the one-question classification rule, and discarded-pass guarantees
- Update the pipeline steps for per-pass collectors, the traversal diagnostics window, and the commit step
- Correct onAppear timing (fires at frame commit, never during traversal) and document change-driven onPreferenceChange semantics
- Compare against the traversal thread's identity instead of Thread.isMainThread, which is not reliable under the Swift concurrency runtime on Linux
@phranck
phranck merged commit da0c247 into main Jul 21, 2026
6 checks passed
@phranck
phranck deleted the issue/58-render-phase-docs branch July 21, 2026 23:16
wadetregaskis added a commit to wadetregaskis/TUIkit that referenced this pull request Sep 2, 2026
…open

PRs phranck#60-phranck#62 are the rest of the render-phase work: per-pass collectors,
an ordered pending-effect log committed from the final traversal, and a
body-mutation diagnostic. Deferred as a real backlog item — it is the
only design that closes the gap a338e81f pins, since the header
correction cannot know which walk is final until after both have run.
Notes record the order to do it in and that it wants Stress --bench on
both sides, given the live perf concern.

PR phranck#63 is queued, and honestly so. Their diagnostic immediately caught
_ImageCore writing its lastSource box on every render; ours does the same
unconditional write, and StateBox.didSet invalidates without comparing.
The suspicion is that a screen with an Image never lets the loop idle.

That is a suspicion, not a finding. A unit test failed to discriminate —
the sink defers to the main-actor frame boundary, so a synchronous
renderToBuffer never reaches the point where anything moves — and the
probe was deleted rather than left asserting something it does not test.
Tools/Profiling/idle_cpu.py exists for exactly this question and is the
right instrument; the one-line fix can follow the measurement rather than
stand in for it.

Co-Authored-By: Claude <[email protected]>
wadetregaskis added a commit to wadetregaskis/TUIkit that referenced this pull request Sep 2, 2026
A write during a walk asks for another frame from inside the frame being
built. Do it unconditionally and the demand-driven loop can never idle —
and because the next frame is usually identical, it burns CPU while
writing nothing. That is exactly what _ImageCore did (75a732fa): 2% CPU,
zero bytes, on a screen showing one image. Finding it took a
purpose-built PTY probe AND a guess about where to look.

This answers the same question by construction. Off by default and free
when off — one optional test on the invalidation path, which is the
single funnel every @State write already reaches. Set
TUIKIT_DIAGNOSE_BODY_MUTATION=1 and reports go to stderr, matching
TUIKIT_DEBUG_RENDER; `reports` collects them in memory for tests and for
an app that wants to surface them itself.

Reported once per FRAME rather than once ever, because that is the
distinction that matters: a write at startup is a one-off, a write on
every frame is the bug.

Keyed on the traversing thread's identity, which is neither of the two
obvious choices, both wrong:

  Thread.isMainThread is unreliable under the Swift concurrency runtime
  on Linux — upstream shipped a fix for exactly that (phranck 8d63dad).

  A @TaskLocal window reads better and is subtly broken: a .task closure
  started DURING the walk inherits the scope, so a legitimate state write
  from that task, running much later, would be reported as a body
  mutation.

Adapted from upstream PR phranck#62 (71629a9), which is where the idea and the
Linux trap both come from. Their version routes into a RuntimeDiagnostics
service and depends on the pass-collector work; this needs none of it,
which is the whole point of taking it separately.

The tests pin what it does NOT report as carefully as what it does: a
read-only view, a write outside any walk (what every event handler does),
and a write from another thread — that last one using
Thread.detachNewThread rather than a Task, since a task can be scheduled
onto the very thread doing the walking and would assert the opposite of
what it means to.

Co-Authored-By: Claude <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P0-08d] Document the phase model and diagnose body side effects

1 participant